fix(backend): make archive and unarchive timestamps monotonic and auditable - #144
Open
woahwhattheheck wants to merge 7 commits into
Open
woahwhattheheck wants to merge 7 commits into
woahwhattheheck wants to merge 7 commits into
Conversation
…itable Archive/unarchive now append immutable history events with actor and reason, keep lastArchivedAt across unarchive, and reject stale expectedUpdatedAt commands so retries and races cannot overwrite lifecycle order.
Archive and unarchive now advance beyond updatedAt, archive history, and the prior archive timestamp. Interleaved claim or cancel operations cannot leave a reused optimistic-concurrency token. Add focused regressions for mixed lifecycle commands and legacy timestamp floors. Validation: npm test (271 passed, 0 failed).
Archive and unarchive passed raw bearer credentials into readable transfer history and audit payloads. Reuse the existing keyed actor fingerprint at the controller boundary, retaining trusted service actor labels and all prior monotonic timestamp, stale-command and retry behavior. Add two real HTTP regressions to the existing lifecycle suite and document actor identity in CHANGELOG. No service, dependency or workflow change. Validation on Node 24.19.0 with all 76 retained package-lock versions matching: - 13 actual local app HTTP requests reproduce raw credential disclosure on parent 0a842a0 and omit both writers' tokens after correction. Same/distinct actors, history/audit consistency, retries, and 409/401/403 controls pass. Invented local fixtures and mock settlement. - Focused archive suites: 36 passed. Parent controller plus new maintained tests: 2 expected HTTP regressions fail, 34 controls pass. - npm test -- --test-concurrency=1: 273 passed, 0 failed, 0 skipped. - git diff --check passes. No install or external settlement. Node 22 hosted CI and maintainer acceptance remain pending. Existing upstream metadata permission denial is retained for the authorized body publisher.
Preserve malformed body concurrency tokens for the existing service freshness check when no nonblank If-Match fallback applies. Keep optional values, body/header selection, actor fingerprints, timestamp ordering, and no-token retry behavior. Add real HTTP regressions to the existing lifecycle file and document the behavior.
Archive and unarchive changed their flags, updatedAt and history before the synchronous audit writer returned. A rejected audit append therefore produced an HTTP failure after changing the transfer and consuming its expectedUpdatedAt token. Perform the existing audit append before committing those state/history changes. Validation, precondition checks, timestamp calculation and existing archive no-op behavior remain in place. Preserve structured-reason validation, actor fingerprints, immutable history fields and the successful response. This protects rejection before an audit write; it is not a durable transaction or rollback guarantee for a writer that partially appends and then throws. Two maintained HTTP regressions inject a pre-write audit failure, read back the unchanged transfer/history/audit, and retry the same token. Each successful retry creates exactly one history event and audit entry with matching actor, reason, request ID and timestamp. Executed node --test test/archiveAuditRetry.test.js with locked npm ci dependencies on Ubuntu 24.04.5 / Node 22.23.3 / npm 10.9.9. Original source dc550d2: 0 pass, 2 assertion failures showing changed state. Repaired source 941667b: 2 pass, 0 fail, 0 skipped. Test blob d03c6f0. Actual application and in-memory audit service with only the controlled failure injected; no full-suite or live-provider claim. Execution: https://github.com/woahwhattheheck/RemitFlow-Backend/actions/runs/37203697476 Artifact archive-audit-retry-before-after, ID 11304325273, SHA256 5621549eb5b7cbd0cd452533dcb38c5ba77b1978210dfe7f441c565ca282ad52. The validation workflow remains on its separate branch; existing CI is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #126.
Repeated archive/unarchive cycles previously cleared
archivedAton unarchive and then wrote a new timestamp on the next archive, which hid earlier lifecycle order and made retention/reconciliation unreliable. This change keeps current-state flags usable while recording an immutable, monotonic event history for every transition.Design
archivedAt(non-null = archived) so list filters stay unchanged.archiveHistoryarray of frozen{ action, at, actor, reason, requestId }events. Event timestamps never move backward and are never rewritten.lastArchivedAtretains the prior archive instant after unarchive so callers can reconcile without digging the full history.expectedUpdatedAt(JSON body or bareIf-Match) rejects stale commands with409 STALE_ARCHIVE_COMMANDso concurrent workers cannot silently overwrite each other.archivedAt/ history length), matching prior behaviour.transfer.archived/transfer.unarchivedentries include actor and reason in the payload.Compatibility
archiveTransfer(id)/unarchiveTransfer(id)call sites keep working.archiveHistory,lastArchivedAt) are additive on transfer objects.Acceptance criteria
active ↔ archivedonly; unarchive when not archived still409archiveHistoryentries;nextTimestampfloor from last eventexpectedUpdatedAt/If-Match→STALE_ARCHIVE_COMMANDtransfer.archived/transfer.unarchivedTest plan
Evidence on this branch: 269/269 passing (includes prior archive suite + new state-machine, stale-command, race, retry, event-order, audit, and regression coverage for the original overwrite/hide failure mode). No unrelated tests skipped or weakened.